Skip to content

Conversation

Rich-Harris
Copy link
Member

@Rich-Harris Rich-Harris commented Sep 29, 2025

This is part of a project to update some of the async batching logic to handle a few edge cases that have cropped up.

The observable change here is that if you have an await expression that runs twice, and the second one somehow resolves before the first, they are no longer forced to resolve in linear order. The easiest way to see it is in this example — click a++ then b++ in quick succession:

  • main — the fast b++ update is blocked on the slow a++ update
  • this PR — the b++ update occurs, followed by the a++ update

To make this work, whenever a batch is committed, we essentially 'rebase' other pending batches on top of the newly applied state. In the example above, when we commit the fast b++ update, we re-run any async effects that depend on b in the context of the pending a++ batch.

Before submitting the PR, please make sure you do the following

  • It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • Prefix your PR title with feat:, fix:, chore:, or docs:.
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.
  • If this PR changes code within packages/svelte/src, add a changeset (npx changeset).

Tests and linting

  • Run the tests with pnpm test and lint the project with pnpm lint

Copy link

changeset-bot bot commented Sep 29, 2025

🦋 Changeset detected

Latest commit: a50d0ec

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
svelte Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@svelte-docs-bot
Copy link

Copy link
Contributor

Playground

pnpm add https://pkg.pr.new/svelte@16866

@Rich-Harris
Copy link
Member Author

moving back to draft, some more improvements incoming

@Rich-Harris Rich-Harris marked this pull request as draft September 29, 2025 20:45
@Rich-Harris Rich-Harris marked this pull request as ready for review September 29, 2025 20:50
@Rich-Harris
Copy link
Member Author

In addition to the tests that are updated in the PR, this fixes the async-redirect test which, for reasons I don't begin to understand, passes on main despite being completely broken. Press 'b', then 'ok', then 'b' again and observe the difference in behaviour:

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks awesome. I'll wait for Simon to review as well before approving; I think everything is good but given he's more familiar with the clientside stuff he should definitely take a look

Comment on lines -18 to -27
// TODO why is this necessary? why isn't `await tick()` enough?
await Promise.resolve();
await Promise.resolve();
await Promise.resolve();
await Promise.resolve();
await Promise.resolve();
await Promise.resolve();
await Promise.resolve();
await Promise.resolve();
flushSync();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😆 I saw this while I was working on async SSR and I was really hopeful we could murder it at some point

Copy link
Member

@dummdidumm dummdidumm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice one!

@dummdidumm dummdidumm merged commit 25cbdc8 into main Sep 30, 2025
18 checks passed
@dummdidumm dummdidumm deleted the merge-batches branch September 30, 2025 13:57
@github-actions github-actions bot mentioned this pull request Sep 29, 2025
dummdidumm added a commit that referenced this pull request Oct 14, 2025
Since #16866, when an async effect runs multiple times, we rebase older batches and rerun those effects. This can have unintended consequences: In a case where an async effect only depends on a single source, and that single source was updated in a later batch, we know that we don't need to / should not rerun the older batch.

This PR makes it so: We collect all the sources of older batches that are not part of the current batch that just committed, and then only mark those async effects as dirty which depend on one of those other sources. Fixes the bug I noticed while working on #16935
dummdidumm added a commit that referenced this pull request Oct 14, 2025
Since #16866, when an async effect runs multiple times, we rebase older batches and rerun those effects. This can have unintended consequences: In a case where an async effect only depends on a single source, and that single source was updated in a later batch, we know that we don't need to / should not rerun the older batch.

This PR makes it so: We collect all the sources of older batches that are not part of the current batch that just committed, and then only mark those async effects as dirty which depend on one of those other sources. Fixes the bug I noticed while working on #16935
Rich-Harris added a commit that referenced this pull request Oct 14, 2025
* runtime-first approach

* revert these

* type safety, lint

* fix: better input cursor restoration for `bind:value` (#16925)

If cursor was at end and new input is longer, move cursor to new end

No test because not possible to reproduce using our test setup.

Follow-up to #14649, helps with #16577

* Version Packages (#16920)

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* docs: await no longer need pending (#16900)

* docs: link to custom renderer issue in Svelte Native discussion (#16896)

* fix code block (#16937)

Updated code block syntax from Svelte to JavaScript for clarity.

* fix: unset context on stale promises (#16935)

* fix: unset context on stale promises

When a stale promise is rejected in `async_derived`, and the promise eventually resolves, `d.resolve` will be noop and `d.promise.then(handler, ...)` will never run. That in turns means any restored context (via `(await save(..))()`) will never be unset. We have to handle this case and unset the context to prevent errors such as false-positive state mutation errors

* fix: unset context on stale promises (slightly different approach) (#16936)

* slightly different approach to #16935

* move unset_context call

* get rid of logs

---------

Co-authored-by: Rich Harris <rich.harris@vercel.com>

* fix: svg `radialGradient` `fr` attribute missing in types (#16943)

* fix(svg radialGradient): fr attribute missing in types

* chore: add changeset

* Version Packages (#16940)

* Version Packages

* Update packages/svelte/CHANGELOG.md

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Rich Harris <rich.harris@vercel.com>

* chore: simplify `batch.apply()` (#16945)

* chore: simplify `batch.apply()`

* belt and braces

* note to self

* unused

* fix: don't rerun async effects unnecessarily (#16944)

Since #16866, when an async effect runs multiple times, we rebase older batches and rerun those effects. This can have unintended consequences: In a case where an async effect only depends on a single source, and that single source was updated in a later batch, we know that we don't need to / should not rerun the older batch.

This PR makes it so: We collect all the sources of older batches that are not part of the current batch that just committed, and then only mark those async effects as dirty which depend on one of those other sources. Fixes the bug I noticed while working on #16935

* fix: ensure map iteration order is correct (#16947)

quick follow-up to #16944
Resetting a map entry does not change its position in the map when iterating. We need to make sure that reset makes that batch jump "to the front" for the "reject all stale batches" logic below. Edge case for which I can't come up with a test case but it _is_ a possibility.

* feat: add `createContext` utility for type-safe context (#16948)

* feat: add `createContext` utility for type-safe context

* regenerate

* Version Packages (#16946)

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* chore: Remove annoying sync-async warning (#16949)

* fix

* use `$state.eager(value)` instead of `$effect.pending(value)`

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Hyunbin Seo <47051820+hyunbinseo@users.noreply.github.com>
Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com>
Co-authored-by: Rich Harris <rich.harris@vercel.com>
Co-authored-by: Hannes Rüger <hannes@hannesrueger.de>
Co-authored-by: Elliott Johnson <sejohnson@torchcloudconsulting.com>
Rich-Harris added a commit that referenced this pull request Oct 18, 2025
* WIP implement `$effect.pending(...)`

* feat: `$state.eager(value)` (#16926)

* runtime-first approach

* revert these

* type safety, lint

* fix: better input cursor restoration for `bind:value` (#16925)

If cursor was at end and new input is longer, move cursor to new end

No test because not possible to reproduce using our test setup.

Follow-up to #14649, helps with #16577

* Version Packages (#16920)

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* docs: await no longer need pending (#16900)

* docs: link to custom renderer issue in Svelte Native discussion (#16896)

* fix code block (#16937)

Updated code block syntax from Svelte to JavaScript for clarity.

* fix: unset context on stale promises (#16935)

* fix: unset context on stale promises

When a stale promise is rejected in `async_derived`, and the promise eventually resolves, `d.resolve` will be noop and `d.promise.then(handler, ...)` will never run. That in turns means any restored context (via `(await save(..))()`) will never be unset. We have to handle this case and unset the context to prevent errors such as false-positive state mutation errors

* fix: unset context on stale promises (slightly different approach) (#16936)

* slightly different approach to #16935

* move unset_context call

* get rid of logs

---------

Co-authored-by: Rich Harris <rich.harris@vercel.com>

* fix: svg `radialGradient` `fr` attribute missing in types (#16943)

* fix(svg radialGradient): fr attribute missing in types

* chore: add changeset

* Version Packages (#16940)

* Version Packages

* Update packages/svelte/CHANGELOG.md

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Rich Harris <rich.harris@vercel.com>

* chore: simplify `batch.apply()` (#16945)

* chore: simplify `batch.apply()`

* belt and braces

* note to self

* unused

* fix: don't rerun async effects unnecessarily (#16944)

Since #16866, when an async effect runs multiple times, we rebase older batches and rerun those effects. This can have unintended consequences: In a case where an async effect only depends on a single source, and that single source was updated in a later batch, we know that we don't need to / should not rerun the older batch.

This PR makes it so: We collect all the sources of older batches that are not part of the current batch that just committed, and then only mark those async effects as dirty which depend on one of those other sources. Fixes the bug I noticed while working on #16935

* fix: ensure map iteration order is correct (#16947)

quick follow-up to #16944
Resetting a map entry does not change its position in the map when iterating. We need to make sure that reset makes that batch jump "to the front" for the "reject all stale batches" logic below. Edge case for which I can't come up with a test case but it _is_ a possibility.

* feat: add `createContext` utility for type-safe context (#16948)

* feat: add `createContext` utility for type-safe context

* regenerate

* Version Packages (#16946)

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* chore: Remove annoying sync-async warning (#16949)

* fix

* use `$state.eager(value)` instead of `$effect.pending(value)`

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Hyunbin Seo <47051820+hyunbinseo@users.noreply.github.com>
Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com>
Co-authored-by: Rich Harris <rich.harris@vercel.com>
Co-authored-by: Hannes Rüger <hannes@hannesrueger.de>
Co-authored-by: Elliott Johnson <sejohnson@torchcloudconsulting.com>

* decouple from boundaries

* use queue_micro_task

* add test

* fix

* changeset

* revert

* tidy up

* update docs

* Update packages/svelte/src/internal/client/reactivity/batch.js

Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com>

* minor tweak

---------

Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Hyunbin Seo <47051820+hyunbinseo@users.noreply.github.com>
Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com>
Co-authored-by: Hannes Rüger <hannes@hannesrueger.de>
Co-authored-by: Elliott Johnson <sejohnson@torchcloudconsulting.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants